Skip to content

[CLEAN] Synthetic Benchmark PR #138557 - Flat index resolution #25

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
tomerqodo wants to merge 1 commit into
base: base_pr_138557_20251204_4885
Choose a base branch
from
Open

[CLEAN] Synthetic Benchmark PR #138557 - Flat index resolution #25

tomerqodo wants to merge 1 commit into from

Conversation

@tomerqodo
Copy link

@tomerqodo tomerqodo commented Dec 4, 2025 •
edited by qodo-code-review bot
Loading

User description

Benchmark PR elastic#138557

Type: Clean (correct implementation)

Original PR Title: Flat index resolution
Original PR Description: Flat index resolution for an esql query. This includes:

  • detecting the query is executed in CPS env
  • performing FC request configured with corresponding flat resolution parameters
  • initialize cross cluster state from the resolved expressions of FC response instead of original indices pattern.
    Original PR URL: Flat index resolution elastic/elasticsearch#138557

PR Type

Enhancement

Description

  • Add flat index resolution for ESQL queries in CPS environments

  • Implement cross-project mode support with flat world indices resolution

  • Refactor index resolution to support both standard and flat world modes

  • Simplify cluster initialization with new initCluster method

Diagram Walkthrough

flowchart LR
  A["ESQL Query"] --> B{Cross Project<br/>Mode Enabled?}
  B -->|No| C["Standard Index<br/>Resolution"]
  B -->|Yes| D["Flat World Index<br/>Resolution"]
  C --> E["Initialize Clusters<br/>from Grouper"]
  D --> F["Initialize Clusters<br/>from FC Response"]
  E --> G["EsqlExecutionInfo"]
  F --> G
Loading

File Walkthrough
Relevant files
Enhancement
EsqlExecutionInfo.java
Add initCluster method for cluster initialization               

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlExecutionInfo.java

  • Add new initCluster method to initialize cluster entries with index
    expression and skip unavailable settings
  • Method consolidates cluster creation logic and handles merging of
    index expressions when cluster already exists
 +13/-0   
EsqlCCSUtils.java
Add EsIndex-based cluster initialization                                 

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlCCSUtils.java

  • Add new overload of initCrossClusterState that accepts EsIndex and
    initializes clusters from resolved indices
  • Simplify existing initCrossClusterState to use new initCluster method
    instead of inline cluster creation
  • Add import for EsIndex class
 +8/-7     
EsqlSession.java
Implement flat world index resolution path                             

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java

  • Add cross-project mode check in preAnalyzeMainIndices to route to
    either standard or flat world resolution
  • Implement new preAnalyzeFlatMainIndices method for flat world index
    resolution
  • Extract query filter creation logic into separate createQueryFilter
    method
  • Simplify preAnalyzeLookupIndex by removing redundant parameters
  • Add thread pool assertion to preAnalyzeMainIndices
 +65/-33 
IndexResolver.java
Refactor index resolution with flat world support               

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/IndexResolver.java

  • Rename FIELD_CAPS_INDICES_OPTIONS to DEFAULT_OPTIONS for clarity
  • Add new FLAT_WORLD_OPTIONS configuration for flat world/CPS context
    resolution
  • Refactor resolveIndices and resolveIndicesVersioned methods with
    simplified signatures
  • Add new resolveFlatWorldIndicesVersioned method for flat world index
    resolution
  • Extract common resolution logic into doResolveIndices method
  • Replace Function parameter with new OriginalIndexExtractor interface
    for better type safety
  • Update createFieldCapsRequest to accept IndicesOptions parameter and
    support includeResolvedTo flag
  • Add DO_NOT_GROUP constant for standard index extraction
 +95/-63 
Refactoring
EnrichPolicyResolver.java
Simplify index resolution call parameters                               

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/EnrichPolicyResolver.java

  • Simplify resolveIndices call by removing redundant parameters (null,
    false, false, false)
  • Reduce method signature complexity while maintaining functionality
 +16/-28 
QueryBuilderResolver.java
Update indices options reference                                                 

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/QueryBuilderResolver.java

  • Change IndexResolver.FIELD_CAPS_INDICES_OPTIONS to
    IndexResolver.DEFAULT_OPTIONS for consistency
 +1/-1     
Tests
AnalyzerTests.java
Update tests to use DO_NOT_GROUP constant                               

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java

  • Update all test calls to IndexResolver.mergedMappings to use
    IndexResolver.DO_NOT_GROUP instead of lambda expressions
  • Maintain test functionality while adapting to new interface
 +11/-11 
EsqlDataTypeRegistryTests.java
Update test to use DO_NOT_GROUP constant                                 

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeRegistryTests.java

  • Update test call to IndexResolver.mergedMappings to use
    IndexResolver.DO_NOT_GROUP instead of lambda
 +1/-1     

@qodo-code-review
Copy link

qodo-code-review bot commented Dec 4, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
⚪
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

⚪
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing Audit Logs: New code paths for flat world/standard index resolution and cluster init do not include
auditing/logging of critical actions, which may be handled elsewhere but is not evident in
the diff.

Referred Code 
private void preAnalyzeMainIndices(
    PreAnalyzer.PreAnalysis preAnalysis,
    EsqlExecutionInfo executionInfo,
    PreAnalysisResult result,
    QueryBuilder requestFilter,
    ActionListener<PreAnalysisResult> listener
) {
    assert ThreadPool.assertCurrentThreadPool(
        ThreadPool.Names.SEARCH,
        ThreadPool.Names.SEARCH_COORDINATION,
        ThreadPool.Names.SYSTEM_READ
    );
    if (crossProjectModeDecider.crossProjectEnabled() == false) {
        EsqlCCSUtils.initCrossClusterState(
            indicesExpressionGrouper,
            verifier.licenseState(),
            preAnalysis.indexes().keySet(),
            executionInfo
        );
        // The main index pattern dictates on which nodes the query can be executed,
        // so we use the minimum transport version from this field caps request.


 ... (clipped 14 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Error Handling Gaps: The new resolve methods delegate failures but add minimal contextual handling (e.g., no
explicit handling for empty/invalid patterns or remote failures beyond passing through),
which may be intentional but is not fully clear from the diff.

Referred Code 
public IndexResolver(Client client) {
    this.client = client;
}

/**
 * Resolves a pattern to one (potentially compound meaning that spawns multiple indices) mapping.
 */
public void resolveIndices(String indexPattern, Set<String> fieldNames, ActionListener<IndexResolution> listener) {
    doResolveIndices(
        createFieldCapsRequest(DEFAULT_OPTIONS, indexPattern, fieldNames, null, false, false),
        indexPattern,
        false,
        false,
        DO_NOT_GROUP,
        listener.map(Versioned::inner)
    );
}

/**
 * Resolves a pattern to one (potentially compound meaning that spawns multiple indices) mapping. Also retrieves the minimum transport
 * version available in the cluster (and remotes).


 ... (clipped 67 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input Validation: New APIs accept index patterns and field sets without visible validation/sanitization in
these changes (relying on underlying Elasticsearch mechanisms), which is likely acceptable
but cannot be confirmed solely from the diff.

Referred Code 
/**
 * Resolves a pattern to one (potentially compound meaning that spawns multiple indices) mapping.
 */
public void resolveIndices(String indexPattern, Set<String> fieldNames, ActionListener<IndexResolution> listener) {
    doResolveIndices(
        createFieldCapsRequest(DEFAULT_OPTIONS, indexPattern, fieldNames, null, false, false),
        indexPattern,
        false,
        false,
        DO_NOT_GROUP,
        listener.map(Versioned::inner)
    );
}

/**
 * Resolves a pattern to one (potentially compound meaning that spawns multiple indices) mapping. Also retrieves the minimum transport
 * version available in the cluster (and remotes).
 */
public void resolveIndicesVersioned(
    String indexPattern,
    Set<String> fieldNames,


 ... (clipped 32 lines)

Learn more about managing compliance generic rules or creating your own custom rules
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

qodo-code-review bot commented Dec 4, 2025

PR Code Suggestions ✨

No code suggestions found for the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Review effort 3/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tomerqodo